Skip to content

API Overhaul #115

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 11 commits into from
Jul 21, 2025
Merged

API Overhaul #115

merged 11 commits into from
Jul 21, 2025

Conversation

@mikeprosserni
Copy link
Collaborator

@mikeprosserni mikeprosserni commented Jul 16, 2025 •
edited
Loading

What does this Pull Request accomplish?

Before sending the API to the API Review Board, we need to polish up the API.

Changes on the ASW side: PR

Proto API Changes:

  • PythonPanelService is now just PanelService, because in the future it could also provide LabVIEW panels.
  • StartPanelRequest now takes a generic panel_configuration instead of anything Streamlit-specific. There is a separate streamlit_panel_configuration.proto that defines the Streamlit-specific configuration parameters.
  • StartPanelResponse now inclueds a URI instead of a URL, since LabVIEW panels might return a file path.

Python API Changes:

  • nipanel.create_panel(panel_script_path) -> nipanel.create_streamlit_panel(panel_script_path)
  • nipanel.get_panel_accessor() -> nipanel.get_streamlit_panel_accessor()
  • the Panel class has been removed. StreamlitPanel inherits from PanelValueAccessor now. All of the functionality in Panel was actually Streamlit-specific, and has been moved into StreamlitPanel instead.
  • the StreamlitPanelValueAccessor class has been removed. The generic PanelValueAccessor can always be used, since it only interacts with the data store, not the panel itself.

Why should this Pull Request be merged?

The API should be improved and reviewed as part of the SEP Review.

What testing has been done?

image

@github-actions
Copy link
Contributor

github-actions bot commented Jul 16, 2025 •
edited
Loading

Test Results

   10 files  ±0     10 suites  ±0   25s ⏱️ -2s
  221 tests ±0    221 ✅ ±0  0 💤 ±0  0 ❌ ±0 
2 160 runs  ±0  2 160 ✅ ±0  0 💤 ±0  0 ❌ ±0 

Results for commit 010d867. ± Comparison against base commit e96fe6e.

This pull request removes 8 and adds 8 tests. Note that renamed tests count towards both. 
tests.unit.test_streamlit_panel_value_accessor ‑ test___no_previous_value___set_value_if_changed___sets_value
tests.unit.test_streamlit_panel_value_accessor ‑ test___set_value_if_changed___different_value_ids___tracks_separately
tests.unit.test_streamlit_panel_value_accessor ‑ test___set_value_if_changed___set_different_value___sets_new_value
tests.unit.test_streamlit_panel_value_accessor ‑ test___set_value_if_changed___set_same_value___does_not_set_value_again
tests.unit.test_streamlit_panel_value_accessor ‑ test___set_value_if_changed_with_enum_value___set_different_value___sets_new_value
tests.unit.test_streamlit_panel_value_accessor ‑ test___set_value_if_changed_with_enum_value___set_same_value___does_not_set_value_again
tests.unit.test_streamlit_panel_value_accessor ‑ test___set_value_if_changed_with_list_value___set_different_value___sets_new_value
tests.unit.test_streamlit_panel_value_accessor ‑ test___set_value_if_changed_with_list_value___set_same_value___does_not_set_value_again

tests.unit.test_panel_value_accessor ‑ test___no_previous_value___set_value_if_changed___sets_value
tests.unit.test_panel_value_accessor ‑ test___set_value_if_changed___different_value_ids___tracks_separately
tests.unit.test_panel_value_accessor ‑ test___set_value_if_changed___set_different_value___sets_new_value
tests.unit.test_panel_value_accessor ‑ test___set_value_if_changed___set_same_value___does_not_set_value_again
tests.unit.test_panel_value_accessor ‑ test___set_value_if_changed_with_enum_value___set_different_value___sets_new_value
tests.unit.test_panel_value_accessor ‑ test___set_value_if_changed_with_enum_value___set_same_value___does_not_set_value_again
tests.unit.test_panel_value_accessor ‑ test___set_value_if_changed_with_list_value___set_different_value___sets_new_value
tests.unit.test_panel_value_accessor ‑ test___set_value_if_changed_with_list_value___set_same_value___does_not_set_value_again

♻️ This comment has been updated with latest results.

@mikeprosserni mikeprosserni marked this pull request as ready for review July 18, 2025 20:49
@mikeprosserni mikeprosserni requested a review from csjall as a code owner July 18, 2025 20:49
Copilot AI reviewed Jul 18, 2025
View reviewed changes
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR performs a comprehensive API overhaul to generalize the panel service from Python-specific to a generic panels API that can support multiple panel types (Streamlit, LabVIEW, etc.). The changes include renaming the service from PythonPanelService to PanelService, introducing generic panel configurations, and updating all function names and class hierarchies to reflect the new generic design.

Key changes:

  • Renamed PythonPanelService to PanelService with generic panel configuration support
  • Updated API functions: create_panel() → create_streamlit_panel(), get_panel_accessor() → get_streamlit_panel_accessor()
  • Consolidated class hierarchy by removing the Panel base class and making StreamlitPanel inherit directly from PanelValueAccessor

Reviewed Changes

Copilot reviewed 51 out of 55 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
tests/utils/_fake_python_panel_servicer.py Updated fake servicer to use new PanelService interfaces and handle generic panel configurations
src/nipanel/_panel_client.py Refactored client to use PanelService stub and generic panel configuration with StreamlitPanelConfiguration
src/nipanel/_streamlit_panel.py Moved panel functionality from removed Panel base class and updated to inherit from PanelValueAccessor
src/nipanel/__init__.py Updated public API exports to reflect renamed functions and classes
src/ni/panels/v1/panel_service_pb2.py New generated protobuf files for the generic PanelService
examples/ Updated all examples to use new API function names
Comments suppressed due to low confidence (1)

jfriedri-ni
jfriedri-ni approved these changes Jul 18, 2025
View reviewed changes
@mikeprosserni mikeprosserni mentioned this pull request Jul 21, 2025
Merged
@mikeprosserni mikeprosserni merged commit e924d0a into main Jul 21, 2025
14 checks passed
@mikeprosserni mikeprosserni deleted the users/mprosser/task-3195342-update-hld branch July 21, 2025 21:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@jfriedri-ni jfriedri-ni jfriedri-ni approved these changes

@csjall csjall Awaiting requested review from csjall csjall is a code owner

@bkeryan bkeryan Awaiting requested review from bkeryan

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@mikeprosserni @jfriedri-ni